Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct typo in docs on acquisition functions #442

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Conversation

Hrovatin
Copy link
Collaborator

@Hrovatin Hrovatin commented Dec 5, 2024

There was a small typo.

Scienfitz
Scienfitz approved these changes Dec 5, 2024
examples/Basics/recommenders.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code has never been turned into an actual meaningful example anyway 😄 but fixing the type is of course still OK 👍🏼

examples/Basics/recommenders.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR :) Just need to fix and rebase the CHANGELOG, other that it's good to go

CHANGELOG.md Outdated Show resolved Hide resolved
@Scienfitz
Copy link
Collaborator

fyi I

  • cleaned the commits, for some reason commits from another unrelated PR were in here
  • removed your changelog entry, the changelog does not need entries for simple typo fixes
  • capitalized commit messages

@Scienfitz Scienfitz added the documentation Improvements or additions to documentation label Dec 12, 2024
@Hrovatin
Copy link
Collaborator Author

fyi I

  • cleaned the commits, for some reason commits from another unrelated PR were in here
  • removed your changelog entry, the changelog does not need entries for simple typo fixes
  • capitalized commit messages

Thank you. I tried to do rebase as it was needed due to changelog, but it seems I messed it up?

@AVHopp the failing test is due to scikit fingerprints right?

@AVHopp
Copy link
Collaborator

AVHopp commented Dec 13, 2024

@Hrovatin , yes it seems like the tests still fail due to the fingerprints error. We could merge the corresponding PR first, then we should be able t verify this.

@Scienfitz Scienfitz merged commit 813ab62 into main Dec 13, 2024
11 checks passed
@Scienfitz
Copy link
Collaborator

@Hrovatin please analyze what happened with the rebase so it doesnt happen again, seems to me maybe it was maybe rebased onto the wrong branch or so.

In any case, be sure to have a backup branch before you do the rebase and let us know if you have any questions / uncertainty around rebasing.

@Scienfitz Scienfitz deleted the fix/docs_acq_typo branch December 13, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants